Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[db_migrator] Set correct CURRENT_VERSION, extend UT #2895

Conversation

vadymhlushko-mlnx
Copy link
Contributor

What I did

Set the correct value for the db_migrator class variable CURRENT_VERSION, because if the new DB version was introduced the CURRENT_VERSION variable should have the newest version value.

How I did it

Edit the db_migrator.py.

How to verify it

Extend existing UT.

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

assert not self.check_config_db(dbmgtr.configDB, expected_db.cfgdb)
assert dbmgtr.CURRENT_VERSION == expected_db.cfgdb.get_entry('VERSIONS', 'DATABASE')['VERSION']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need a dedicated test just for checking this assertion.

My goal is to use UT to always catch the issue that this PR is addressing -- missing update to CURRENT_VERSION

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how to achieve that, because not every UT mocks the config_db with a VERSIONS key

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. May be we should always verify this assertion in the new UTs that we add. Specifically, in the changes where new version is introduced.

We can have that conversation later, let's take this bug fix for now.

@vaibhavhd vaibhavhd merged commit 61bad06 into sonic-net:master Jul 6, 2023
@vaibhavhd
Copy link
Contributor

MSFT ADO : 24471596

dgsudharsan added a commit to dgsudharsan/sonic-buildimage that referenced this pull request Jul 11, 2023
Update sonic-utilities submodule pointer to include the following:
* ff380e04 [hash]: Implement GH frontend ([sonic-net#2580](sonic-net/sonic-utilities#2580))
* 61bad064 [db_migrator] Set correct CURRENT_VERSION, extend UT ([sonic-net#2895](sonic-net/sonic-utilities#2895))
* 6b8ee47c [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([sonic-net#2885](sonic-net/sonic-utilities#2885))
* 73d8d633 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([sonic-net#2750](sonic-net/sonic-utilities#2750))
* 7bc08c28 [db_migrator] Remove hardcoded config and migrate config from minigraph ([sonic-net#2887](sonic-net/sonic-utilities#2887))
* b1aa9426 [generate_dump]: Enhance show techsupport for Marvell platform ([sonic-net#2676](sonic-net/sonic-utilities#2676))
* 316b14c0 Add support for secure upgrade ([sonic-net#2698](sonic-net/sonic-utilities#2698))
* dc2945bc [dns] Implement config and show commands for static DNS. ([sonic-net#2737](sonic-net/sonic-utilities#2737))
* 8414a709 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([sonic-net#2525](sonic-net/sonic-utilities#2525))
* 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882))
* 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([sonic-net#2839](sonic-net/sonic-utilities#2839))
* fceef2ed [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789))

Signed-off-by: dgsudharsan <[email protected]>
liat-grozovik pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Jul 11, 2023
Update sonic-utilities submodule pointer to include the following:
* ff380e04 [hash]: Implement GH frontend ([#2580](sonic-net/sonic-utilities#2580))
* 61bad064 [db_migrator] Set correct CURRENT_VERSION, extend UT ([#2895](sonic-net/sonic-utilities#2895))
* 6b8ee47c [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([#2885](sonic-net/sonic-utilities#2885))
* 73d8d633 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([#2750](sonic-net/sonic-utilities#2750))
* 7bc08c28 [db_migrator] Remove hardcoded config and migrate config from minigraph ([#2887](sonic-net/sonic-utilities#2887))
* b1aa9426 [generate_dump]: Enhance show techsupport for Marvell platform ([#2676](sonic-net/sonic-utilities#2676))
* 316b14c0 Add support for secure upgrade ([#2698](sonic-net/sonic-utilities#2698))
* dc2945bc [dns] Implement config and show commands for static DNS. ([#2737](sonic-net/sonic-utilities#2737))
* 8414a709 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([#2525](sonic-net/sonic-utilities#2525))
* 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([#2882](sonic-net/sonic-utilities#2882))
* 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([#2839](sonic-net/sonic-utilities#2839))
* fceef2ed [chassis][voq] Clear fabric counters queue/port ([#2789](sonic-net/sonic-utilities#2789))

Signed-off-by: dgsudharsan <[email protected]>
@bingwang-ms
Copy link
Contributor

@StormLiangMS Please help cherry-pick this PR to 202305. Thanks!

StormLiangMS pushed a commit that referenced this pull request Aug 14, 2023
Signed-off-by: vadymhlushko-mlnx <[email protected]>

What I did
Set the correct value for the db_migrator class variable CURRENT_VERSION, because if the new DB version was introduced the CURRENT_VERSION variable should have the newest version value.

How I did it
Edit the db_migrator.py.

How to verify it
Extend existing UT.
pdhruv-marvell pushed a commit to pdhruv-marvell/sonic-utilities that referenced this pull request Aug 23, 2023
Signed-off-by: vadymhlushko-mlnx <[email protected]>

What I did
Set the correct value for the db_migrator class variable CURRENT_VERSION, because if the new DB version was introduced the CURRENT_VERSION variable should have the newest version value.

How I did it
Edit the db_migrator.py.

How to verify it
Extend existing UT.
sonic-otn pushed a commit to sonic-otn/sonic-buildimage that referenced this pull request Sep 20, 2023
Update sonic-utilities submodule pointer to include the following:
* ff380e04 [hash]: Implement GH frontend ([sonic-net#2580](sonic-net/sonic-utilities#2580))
* 61bad064 [db_migrator] Set correct CURRENT_VERSION, extend UT ([sonic-net#2895](sonic-net/sonic-utilities#2895))
* 6b8ee47c [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([sonic-net#2885](sonic-net/sonic-utilities#2885))
* 73d8d633 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([sonic-net#2750](sonic-net/sonic-utilities#2750))
* 7bc08c28 [db_migrator] Remove hardcoded config and migrate config from minigraph ([sonic-net#2887](sonic-net/sonic-utilities#2887))
* b1aa9426 [generate_dump]: Enhance show techsupport for Marvell platform ([sonic-net#2676](sonic-net/sonic-utilities#2676))
* 316b14c0 Add support for secure upgrade ([sonic-net#2698](sonic-net/sonic-utilities#2698))
* dc2945bc [dns] Implement config and show commands for static DNS. ([sonic-net#2737](sonic-net/sonic-utilities#2737))
* 8414a709 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([sonic-net#2525](sonic-net/sonic-utilities#2525))
* 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882))
* 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([sonic-net#2839](sonic-net/sonic-utilities#2839))
* fceef2ed [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789))

Signed-off-by: dgsudharsan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants